Skip to content

Automated Test: workflow-queue-enhanced #381

Closed

Conversation

admin-coderabbit
Copy link
Owner

@admin-coderabbit admin-coderabbit commented Feb 4, 2026

This pull request was automatically created by @coderabbitai/e2e-reviewer.

Batch created pull request.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved reliability of workflow reminder cancellation when bookings are cancelled or rescheduled.
    • Enhanced phone number verification process in workflow configuration with a streamlined verification code input flow.
  • Refactor

    • Optimized reminder deletion operations for better performance and state tracking.

…re still sent (#7232)

* small UI fix

* fix cancelling scheduled emails

* improve comments

* delete reminders for rescheduled bookings

* add migration file

* cancel rescheduled bookings immediately

* remove immediate delete for request reschedule

---------

Co-authored-by: CarinaWolli <wollencarina@gmail.com>
@coderabbit-eval
Copy link

coderabbit-eval bot commented Feb 4, 2026

📝 Walkthrough

Walkthrough

The changes introduce a new cancelled field to the WorkflowReminder model to track cancellation status, update reminder manager function signatures to accept reminder IDs and implement conditional deletion logic, and refactor reminder deletion across booking handlers and API endpoints from batch Prisma operations to direct function calls. The phone verification UI flow in the workflow component is also simplified.

Changes

Cohort / File(s) Summary
Database Schema & Migrations
packages/prisma/schema.prisma, packages/prisma/migrations/20230217230604_add_cancelled_to_workflow_reminder/migration.sql
Added cancelled boolean field to WorkflowReminder model to track reminder cancellation status.
Reminder Manager Functions
packages/features/ee/workflows/lib/reminders/emailReminderManager.ts, packages/features/ee/workflows/lib/reminders/smsReminderManager.ts
Updated function signatures: deleteScheduledEmailReminder(reminderId, referenceId, immediateDelete?) and deleteScheduledSMSReminder(reminderId, referenceId). Refactored logic to handle conditional API cancellation, database flagging via cancelled field, or direct deletion based on input parameters.
Booking Handler Functions
packages/features/bookings/lib/handleCancelBooking.ts, packages/features/bookings/lib/handleNewBooking.ts
Refactored reminder deletion from batched Prisma operations to direct function calls. Added workflow reminder cancellation during rescheduling with try/catch error handling. Updated calls to match new reminder manager signatures with (reminder.id, reminder.referenceId).
API Endpoints & TRPC Routers
packages/features/ee/workflows/api/scheduleEmailReminders.ts, packages/trpc/server/routers/viewer/bookings.tsx, packages/trpc/server/routers/viewer/workflows.tsx
Introduced new email reminder cancellation workflow that queries for cancelled reminders and sends batch cancellation requests to external API. Updated all reminder deletion call sites to use new function signatures with reminder IDs and conditional flags.
Workflow UI Component
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
Simplified phone number verification flow: replaced combined phone-input/send-code block with dedicated verification input field and verify button. Removed inline code verification logic, keeping state management but decoupling from immediate onChange handling.

Sequence Diagram(s)

sequenceDiagram
    participant Scheduler as Email Reminder<br/>Scheduler
    participant DB as Database
    participant API as External API<br/>(Sendgrid)
    participant Queue as Job Queue

    Scheduler->>DB: Query cancelled reminders<br/>(cancelled=true, scheduled<1hr)
    DB-->>Scheduler: Return cancelled reminders
    
    loop For each cancelled reminder
        Scheduler->>API: POST /v3/user/scheduled_sends<br/>(batch_id, status="cancel")
        API-->>Scheduler: Confirmation
        Scheduler->>Queue: Queue reminder deletion task
    end
    
    Scheduler->>Queue: Await all deletions
    Queue->>DB: Delete reminder records
    DB-->>Queue: Deletion confirmed
    Queue-->>Scheduler: Complete
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Reminders now cancel with grace and with care,
New IDs in hand, deleted fair and square,
Database flags track what's been slain,
Phone checks simplified, no more the strain,
Workflow's refined from tip to its tail! 🌟

🚥 Pre-merge checks | ✅ 1 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title 'Automated Test: workflow-queue-enhanced' does not accurately reflect the substantive changes in the changeset, which involve refactoring workflow reminder deletion logic, updating function signatures, and adding a cancelled flag to the WorkflowReminder model. Update the title to clearly describe the main changes, such as 'Refactor workflow reminder deletion to use direct function calls' or 'Add cancelled flag and update reminder deletion logic'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch workflow-queue-enhanced

Comment @coderabbit eval help to get the list of available commands and usage tips.

@coderabbit-eval coderabbit-eval bot closed this Feb 4, 2026
Copy link

@coderabbit-eval coderabbit-eval bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/features/ee/workflows/lib/reminders/emailReminderManager.ts (1)

197-235: ⚠️ Potential issue | 🟠 Major

Reschedule scenario orphans reminder records with immediateDelete=true.

The three paths are confirmed:

  1. No referenceId → delete DB record (unscheduled reminder).
  2. immediateDelete=true → cancel via SendGrid API only, no DB mutation.
  3. Default → set cancelled = true for deferred CRON processing.

Path 2 works when the caller immediately cascade-deletes the parent (workflow/step deletion), but in the reschedule flow (handleNewBooking.ts:968), the original booking is not deleted. The reminder record is left orphaned with cancelled still null and scheduled still true. The record will persist in the database until its scheduledDate passes, at which point the CRON job's cleanup query (scheduledDate <= now()) removes it. This creates a window of orphaned records.

Consider either setting cancelled = true before the SendGrid API call in reschedule scenarios, or document why the temporary orphaning is acceptable given the CRON cleanup behavior.

🤖 Fix all issues with AI agents
In `@packages/features/bookings/lib/handleCancelBooking.ts`:
- Around line 484-497: The reminder deletion calls in the updatedBookings loop
are fire-and-forget; collect their returned promises and include them in the
awaited batch: when iterating updatedBookings and booking.workflowReminders,
push the promises returned by deleteScheduledEmailReminder(reminder.id,
reminder.referenceId) and deleteScheduledSMSReminder(reminder.id,
reminder.referenceId) into the prismaPromises array (or a new array merged into
prismaPromises) so that the final await
Promise.all(prismaPromises.concat(apiDeletes)) awaits the reminder deletions as
well.

In `@packages/features/bookings/lib/handleNewBooking.ts`:
- Around line 964-975: The current try/catch around
originalRescheduledBooking.workflowReminders.forEach(...) calls async functions
deleteScheduledEmailReminder and deleteScheduledSMSReminder without awaiting
them, so promise rejections are lost; replace the forEach with a mapped array of
promises (map over originalRescheduledBooking.workflowReminders and for each
reminder return the appropriate async call based on WorkflowMethods.EMAIL /
WorkflowMethods.SMS), then await Promise.allSettled(...) (or Promise.all(...) if
you prefer failing fast) and inspect the results to log any failures via
log.error so rejections are not silently swallowed; keep the surrounding
try/catch for synchronous errors but handle async results explicitly.

In `@packages/features/ee/workflows/api/scheduleEmailReminders.ts`:
- Around line 43-77: The query using prisma.workflowReminder.findMany must
exclude reminders with null referenceId and the SendGrid cancellation must not
abort the whole batch: update the findMany where clause to add referenceId: {
not: null } so reminder.referenceId is always defined, and change the for-loop
so each reminder cancellation is wrapped in its own try/catch (or use
per-reminder promise handling) — only push the corresponding
prisma.workflowReminder.delete(...) into workflowRemindersToDelete after a
successful client.request call; after the loop await
Promise.all(workflowRemindersToDelete) as before and log per-reminder errors
from client.request without breaking the loop (references:
prisma.workflowReminder.findMany, reminder.referenceId, client.request,
workflowRemindersToDelete, prisma.workflowReminder.delete, Promise.all).

In `@packages/features/ee/workflows/components/WorkflowStepContainer.tsx`:
- Around line 436-457: The placeholder "Verification code" on the TextField and
the "Verify" label on the Button are hardcoded and must be internationalized;
update the JSX in WorkflowStepContainer so both strings use the translation
function (t) like other user-facing text in this component (apply t("...") to
the TextField placeholder and to the Button children), keeping the existing
state/handler usage (verificationCode, setVerificationCode,
verifyPhoneNumberMutation.mutate, form.getValues) unchanged.

In `@packages/trpc/server/routers/viewer/bookings.tsx`:
- Around line 487-494: The cancellation of workflow reminders on
bookingToReschedule currently uses forEach and fires
deleteScheduledEmailReminder/deleteScheduledSMSReminder without awaiting them,
so ensure these async cancellations complete by collecting and awaiting the
promises (e.g., map to promises and await Promise.all(...) or use a for..of with
await) when method === WorkflowMethods.EMAIL call
deleteScheduledEmailReminder(reminder.id, reminder.referenceId) and when ===
WorkflowMethods.SMS call deleteScheduledSMSReminder(reminder.id,
reminder.referenceId); update the code around
bookingToReschedule.workflowReminders to await those calls before returning the
response.

In `@packages/trpc/server/routers/viewer/workflows.tsx`:
- Around line 572-580: The current remindersToUpdate.forEach(async (reminder) =>
{ ... }) uses an async callback without awaiting the promises, causing
fire-and-forget behavior and unhandled errors; change this to either await
Promise.all(remindersToUpdate.map(async reminder => { ... })) or use a for...of
loop and await each call, and wrap
deleteScheduledEmailReminder/deleteScheduledSMSReminder invocations in try/catch
to surface/log errors (refer to remindersToUpdate,
WorkflowMethods.EMAIL/WorkflowMethods.SMS, deleteScheduledEmailReminder,
deleteScheduledSMSReminder).
- Around line 375-382: The current forEach on remindersToDelete drops promises
from deleteScheduledEmailReminder/deleteScheduledSMSReminder (used with the
default non-immediateDelete path that sets cancelled = true), which can cause
the DB write to be lost; replace the forEach with mapping to an array of
promises and await them (e.g. await
Promise.all(remindersToDelete.flat().map(...))) so that calls to
deleteScheduledEmailReminder and deleteScheduledSMSReminder are awaited and
complete before continuing; keep using WorkflowMethods to branch but ensure you
return the promise from each call so Promise.all receives them.
- Around line 516-526: The current fire-and-forget loop over remindersFromStep
using forEach drops promises and races with the cascade delete; replace the
forEach with building an array of promises (map each reminder to calling
deleteScheduledEmailReminder or deleteScheduledSMSReminder) and await
Promise.all(promises) before proceeding to cascade-delete the step (ensure the
surrounding function is async and any call sites handle the awaited result);
reference remindersFromStep, newStep, WorkflowMethods,
deleteScheduledEmailReminder, and deleteScheduledSMSReminder when making this
change.
- Around line 211-218: The cancellation calls to deleteScheduledEmailReminder
and deleteScheduledSMSReminder are being fire-and-forget inside
scheduledReminders.forEach, creating a race with the subsequent
workflow.deleteMany cascade; collect the promises (e.g., map scheduledReminders
to calls to deleteScheduledEmailReminder/deleteScheduledSMSReminder) and await
Promise.all on them before calling workflow.deleteMany so all external scheduler
cancellations complete first; ensure you reference WorkflowMethods,
deleteScheduledEmailReminder, deleteScheduledSMSReminder and the
workflow.deleteMany call when making the change so the SMS path which also
removes DB rows won't race with the cascade delete.
🧹 Nitpick comments (3)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (1)

401-406: Unnecessary array copy before .find().

.concat([]) creates a shallow copy of verifiedNumbers, but .find() is a read-only operation that doesn't mutate the array. You can call .find() directly.

♻️ Suggested simplification
                      onChange={() => {
-                       const isAlreadyVerified = !!verifiedNumbers
-                         ?.concat([])
-                         .find((number) => number === form.getValues(`steps.${step.stepNumber - 1}.sendTo`));
+                       const isAlreadyVerified = !!verifiedNumbers
+                         ?.find((number) => number === form.getValues(`steps.${step.stepNumber - 1}.sendTo`));
                        setNumberVerified(isAlreadyVerified);
                      }}
packages/features/ee/workflows/lib/reminders/smsReminderManager.ts (1)

177-189: Consistent per-reminder deletion pattern for SMS.

The logic correctly gates the Twilio cancellation on referenceId presence and always attempts DB cleanup. Note that if twilio.cancelSMS throws, the prisma.workflowReminder.delete on line 182 is skipped, leaving an orphaned DB record. This matches the pre-existing error-handling pattern, but consider separating the two operations so that the DB record is cleaned up even if the external call fails.

♻️ Optional: isolate external call failure from DB cleanup
 export const deleteScheduledSMSReminder = async (reminderId: number, referenceId: string | null) => {
-  try {
-    if (referenceId) {
+  if (referenceId) {
+    try {
       await twilio.cancelSMS(referenceId);
+    } catch (error) {
+      console.log(`Error canceling SMS via Twilio with error ${error}`);
     }
+  }
+  try {
     await prisma.workflowReminder.delete({
       where: {
         id: reminderId,
       },
     });
   } catch (error) {
-    console.log(`Error canceling reminder with error ${error}`);
+    console.log(`Error deleting reminder record with error ${error}`);
   }
 };
packages/features/ee/workflows/api/scheduleEmailReminders.ts (1)

54-54: Avoid using Prisma internal type Prisma__WorkflowReminderClient.

Prisma.Prisma__WorkflowReminderClient<WorkflowReminder, never> is an internal implementation type that may change across Prisma versions. Use PrismaPromise<WorkflowReminder> instead (already imported in workflows.tsx).

Comment on lines +484 to 497
//Workflows - cancel all reminders for cancelled bookings
updatedBookings.forEach((booking) => {
booking.workflowReminders.forEach((reminder) => {
if (reminder.scheduled && reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
const reminderToDelete = prisma.workflowReminder.deleteMany({
where: {
id: reminder.id,
},
});
remindersToDelete.push(reminderToDelete);
});
});

const prismaPromises: Promise<unknown>[] = [attendeeDeletes, bookingReferenceDeletes].concat(
remindersToDelete
);
const prismaPromises: Promise<unknown>[] = [attendeeDeletes, bookingReferenceDeletes];

await Promise.all(prismaPromises.concat(apiDeletes));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Same unawaited-async issue: reminder deletion promises are fire-and-forget.

The deleteScheduledEmailReminder / deleteScheduledSMSReminder calls return promises that are never awaited or collected. They could silently fail. Since prismaPromises is already collected and awaited on line 497, push the deletion promises into it.

🔧 Proposed fix — collect and await deletion promises
   //Workflows - cancel all reminders for cancelled bookings
+  const reminderDeletionPromises: Promise<unknown>[] = [];
   updatedBookings.forEach((booking) => {
     booking.workflowReminders.forEach((reminder) => {
       if (reminder.method === WorkflowMethods.EMAIL) {
-        deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
+        reminderDeletionPromises.push(deleteScheduledEmailReminder(reminder.id, reminder.referenceId));
       } else if (reminder.method === WorkflowMethods.SMS) {
-        deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
+        reminderDeletionPromises.push(deleteScheduledSMSReminder(reminder.id, reminder.referenceId));
       }
     });
   });

-  const prismaPromises: Promise<unknown>[] = [attendeeDeletes, bookingReferenceDeletes];
+  const prismaPromises: Promise<unknown>[] = [attendeeDeletes, bookingReferenceDeletes, ...reminderDeletionPromises];
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
//Workflows - cancel all reminders for cancelled bookings
updatedBookings.forEach((booking) => {
booking.workflowReminders.forEach((reminder) => {
if (reminder.scheduled && reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
const reminderToDelete = prisma.workflowReminder.deleteMany({
where: {
id: reminder.id,
},
});
remindersToDelete.push(reminderToDelete);
});
});
const prismaPromises: Promise<unknown>[] = [attendeeDeletes, bookingReferenceDeletes].concat(
remindersToDelete
);
const prismaPromises: Promise<unknown>[] = [attendeeDeletes, bookingReferenceDeletes];
await Promise.all(prismaPromises.concat(apiDeletes));
//Workflows - cancel all reminders for cancelled bookings
const reminderDeletionPromises: Promise<unknown>[] = [];
updatedBookings.forEach((booking) => {
booking.workflowReminders.forEach((reminder) => {
if (reminder.method === WorkflowMethods.EMAIL) {
reminderDeletionPromises.push(deleteScheduledEmailReminder(reminder.id, reminder.referenceId));
} else if (reminder.method === WorkflowMethods.SMS) {
reminderDeletionPromises.push(deleteScheduledSMSReminder(reminder.id, reminder.referenceId));
}
});
});
const prismaPromises: Promise<unknown>[] = [attendeeDeletes, bookingReferenceDeletes, ...reminderDeletionPromises];
await Promise.all(prismaPromises.concat(apiDeletes));
🤖 Prompt for AI Agents
In `@packages/features/bookings/lib/handleCancelBooking.ts` around lines 484 -
497, The reminder deletion calls in the updatedBookings loop are
fire-and-forget; collect their returned promises and include them in the awaited
batch: when iterating updatedBookings and booking.workflowReminders, push the
promises returned by deleteScheduledEmailReminder(reminder.id,
reminder.referenceId) and deleteScheduledSMSReminder(reminder.id,
reminder.referenceId) into the prismaPromises array (or a new array merged into
prismaPromises) so that the final await
Promise.all(prismaPromises.concat(apiDeletes)) awaits the reminder deletions as
well.

Comment on lines +964 to +975
try {
// cancel workflow reminders from previous rescheduled booking
originalRescheduledBooking.workflowReminders.forEach((reminder) => {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
});
} catch (error) {
log.error("Error while canceling scheduled workflow reminders", error);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the function signatures of the deletion helpers
echo "=== Searching for deleteScheduledEmailReminder ==="
rg -n "deleteScheduledEmailReminder" --type=ts -B 2 -A 5

echo ""
echo "=== Searching for deleteScheduledSMSReminder ==="
rg -n "deleteScheduledSMSReminder" --type=ts -B 2 -A 5

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 20325


🏁 Script executed:

#!/bin/bash
# Check the specific file and lines mentioned in the review
echo "=== Checking the target file at lines 964-975 ==="
sed -n '960,980p' packages/features/bookings/lib/handleNewBooking.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 932


Fix unawaited async calls in forEach loop — errors silently swallowed.

deleteScheduledEmailReminder and deleteScheduledSMSReminder are async functions, but they're called without await inside .forEach(). The surrounding try/catch only catches synchronous exceptions, not rejections from the unawaited promises. Failed deletions will be silently lost rather than logged.

Use Promise.allSettled or Promise.all with .map() to properly await all deletions:

Proposed fix
     try {
       // cancel workflow reminders from previous rescheduled booking
-      originalRescheduledBooking.workflowReminders.forEach((reminder) => {
+      await Promise.all(originalRescheduledBooking.workflowReminders.map(async (reminder) => {
         if (reminder.method === WorkflowMethods.EMAIL) {
-          deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
+          await deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
         } else if (reminder.method === WorkflowMethods.SMS) {
-          deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
+          await deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
         }
-      });
+      }));
     } catch (error) {
       log.error("Error while canceling scheduled workflow reminders", error);
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try {
// cancel workflow reminders from previous rescheduled booking
originalRescheduledBooking.workflowReminders.forEach((reminder) => {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
});
} catch (error) {
log.error("Error while canceling scheduled workflow reminders", error);
}
try {
// cancel workflow reminders from previous rescheduled booking
await Promise.all(originalRescheduledBooking.workflowReminders.map(async (reminder) => {
if (reminder.method === WorkflowMethods.EMAIL) {
await deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
} else if (reminder.method === WorkflowMethods.SMS) {
await deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
}));
} catch (error) {
log.error("Error while canceling scheduled workflow reminders", error);
}
🤖 Prompt for AI Agents
In `@packages/features/bookings/lib/handleNewBooking.ts` around lines 964 - 975,
The current try/catch around
originalRescheduledBooking.workflowReminders.forEach(...) calls async functions
deleteScheduledEmailReminder and deleteScheduledSMSReminder without awaiting
them, so promise rejections are lost; replace the forEach with a mapped array of
promises (map over originalRescheduledBooking.workflowReminders and for each
reminder return the appropriate async call based on WorkflowMethods.EMAIL /
WorkflowMethods.SMS), then await Promise.allSettled(...) (or Promise.all(...) if
you prefer failing fast) and inspect the results to log any failures via
log.error so rejections are not silently swallowed; keep the surrounding
try/catch for synchronous errors but handle async results explicitly.

Comment on lines +43 to +77
//cancel reminders for cancelled/rescheduled bookings that are scheduled within the next hour
const remindersToCancel = await prisma.workflowReminder.findMany({
where: {
cancelled: true,
scheduledDate: {
lte: dayjs().add(1, "hour").toISOString(),
},
},
});

try {
const workflowRemindersToDelete: Prisma.Prisma__WorkflowReminderClient<WorkflowReminder, never>[] = [];

for (const reminder of remindersToCancel) {
await client.request({
url: "/v3/user/scheduled_sends",
method: "POST",
body: {
batch_id: reminder.referenceId,
status: "cancel",
},
});

const workflowReminderToDelete = prisma.workflowReminder.delete({
where: {
id: reminder.id,
},
});

workflowRemindersToDelete.push(workflowReminderToDelete);
}
await Promise.all(workflowRemindersToDelete);
} catch (error) {
console.log(`Error cancelling scheduled Emails: ${error}`);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Null referenceId will be sent to SendGrid as batch_id, and a single failure aborts the entire batch.

Two issues:

  1. The query on line 44 doesn't filter out referenceId: null, so reminder.referenceId (line 61) can be null, sending an invalid batch_id to the SendGrid API.
  2. If the SendGrid call throws for any reminder, the for loop exits to catch, skipping cancellation of remaining reminders and skipping Promise.all on line 74 — so already-cancelled reminders aren't deleted from the DB either.
🐛 Proposed fix: filter out null referenceIds and handle per-reminder errors
   const remindersToCancel = await prisma.workflowReminder.findMany({
     where: {
       cancelled: true,
+      referenceId: { not: null },
       scheduledDate: {
         lte: dayjs().add(1, "hour").toISOString(),
       },
     },
   });

-  try {
-    const workflowRemindersToDelete: Prisma.Prisma__WorkflowReminderClient<WorkflowReminder, never>[] = [];
-
-    for (const reminder of remindersToCancel) {
+  const deletePromises: Promise<unknown>[] = [];
+  for (const reminder of remindersToCancel) {
+    try {
       await client.request({
         url: "/v3/user/scheduled_sends",
         method: "POST",
         body: {
           batch_id: reminder.referenceId,
           status: "cancel",
         },
       });
-
-      const workflowReminderToDelete = prisma.workflowReminder.delete({
+    } catch (error) {
+      console.log(`Error cancelling scheduled Email for reminder ${reminder.id}: ${error}`);
+    }
+    deletePromises.push(
+      prisma.workflowReminder.delete({
         where: {
           id: reminder.id,
         },
-      });
-
-      workflowRemindersToDelete.push(workflowReminderToDelete);
-    }
-    await Promise.all(workflowRemindersToDelete);
-  } catch (error) {
-    console.log(`Error cancelling scheduled Emails: ${error}`);
+      })
+    );
   }
+  try {
+    await Promise.all(deletePromises);
+  } catch (error) {
+    console.log(`Error deleting cancelled reminders: ${error}`);
+  }
🤖 Prompt for AI Agents
In `@packages/features/ee/workflows/api/scheduleEmailReminders.ts` around lines 43
- 77, The query using prisma.workflowReminder.findMany must exclude reminders
with null referenceId and the SendGrid cancellation must not abort the whole
batch: update the findMany where clause to add referenceId: { not: null } so
reminder.referenceId is always defined, and change the for-loop so each reminder
cancellation is wrapped in its own try/catch (or use per-reminder promise
handling) — only push the corresponding prisma.workflowReminder.delete(...) into
workflowRemindersToDelete after a successful client.request call; after the loop
await Promise.all(workflowRemindersToDelete) as before and log per-reminder
errors from client.request without breaking the loop (references:
prisma.workflowReminder.findMany, reminder.referenceId, client.request,
workflowRemindersToDelete, prisma.workflowReminder.delete, Promise.all).

Comment on lines +436 to 457
<div className="mt-3 flex">
<TextField
className=" border-r-transparent"
placeholder="Verification code"
value={verificationCode}
onChange={(e) => {
setVerificationCode(e.target.value);
}}
required
/>
<Button
color="secondary"
disabled={numberVerified || false}
className={classNames(
"-ml-[3px] h-[40px] min-w-fit sm:block sm:rounded-tl-none sm:rounded-bl-none ",
numberVerified ? "hidden" : "mt-3 sm:mt-0"
)}
onClick={() =>
sendVerificationCodeMutation.mutate({
className="-ml-[3px] rounded-tl-none rounded-bl-none "
disabled={verifyPhoneNumberMutation.isLoading}
onClick={() => {
verifyPhoneNumberMutation.mutate({
phoneNumber: form.getValues(`steps.${step.stepNumber - 1}.sendTo`) || "",
})
}>
{t("send_code")}
code: verificationCode,
});
}}>
Verify
</Button>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Hardcoded English strings break i18n.

Line 439 ("Verification code") and Line 456 ("Verify") are not wrapped in t(), unlike every other user-facing string in this component. These will render as English regardless of locale.

🌐 Proposed fix
                        <TextField
                          className=" border-r-transparent"
-                         placeholder="Verification code"
+                         placeholder={t("verification_code")}
                          value={verificationCode}
                          onChange={(e) => {
                            setVerificationCode(e.target.value);
                          }}
                          required
                        />
                        <Button
                          color="secondary"
                          className="-ml-[3px] rounded-tl-none rounded-bl-none "
                          disabled={verifyPhoneNumberMutation.isLoading}
                          onClick={() => {
                            verifyPhoneNumberMutation.mutate({
                              phoneNumber: form.getValues(`steps.${step.stepNumber - 1}.sendTo`) || "",
                              code: verificationCode,
                            });
                          }}>
-                         Verify
+                         {t("verify")}
                        </Button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div className="mt-3 flex">
<TextField
className=" border-r-transparent"
placeholder="Verification code"
value={verificationCode}
onChange={(e) => {
setVerificationCode(e.target.value);
}}
required
/>
<Button
color="secondary"
disabled={numberVerified || false}
className={classNames(
"-ml-[3px] h-[40px] min-w-fit sm:block sm:rounded-tl-none sm:rounded-bl-none ",
numberVerified ? "hidden" : "mt-3 sm:mt-0"
)}
onClick={() =>
sendVerificationCodeMutation.mutate({
className="-ml-[3px] rounded-tl-none rounded-bl-none "
disabled={verifyPhoneNumberMutation.isLoading}
onClick={() => {
verifyPhoneNumberMutation.mutate({
phoneNumber: form.getValues(`steps.${step.stepNumber - 1}.sendTo`) || "",
})
}>
{t("send_code")}
code: verificationCode,
});
}}>
Verify
</Button>
<div className="mt-3 flex">
<TextField
className=" border-r-transparent"
placeholder={t("verification_code")}
value={verificationCode}
onChange={(e) => {
setVerificationCode(e.target.value);
}}
required
/>
<Button
color="secondary"
className="-ml-[3px] rounded-tl-none rounded-bl-none "
disabled={verifyPhoneNumberMutation.isLoading}
onClick={() => {
verifyPhoneNumberMutation.mutate({
phoneNumber: form.getValues(`steps.${step.stepNumber - 1}.sendTo`) || "",
code: verificationCode,
});
}}>
{t("verify")}
</Button>
🤖 Prompt for AI Agents
In `@packages/features/ee/workflows/components/WorkflowStepContainer.tsx` around
lines 436 - 457, The placeholder "Verification code" on the TextField and the
"Verify" label on the Button are hardcoded and must be internationalized; update
the JSX in WorkflowStepContainer so both strings use the translation function
(t) like other user-facing text in this component (apply t("...") to the
TextField placeholder and to the Button children), keeping the existing
state/handler usage (verificationCode, setVerificationCode,
verifyPhoneNumberMutation.mutate, form.getValues) unchanged.

Comment on lines +487 to 494
//cancel workflow reminders of previous booking
bookingToReschedule.workflowReminders.forEach((reminder) => {
if (reminder.scheduled && reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
const reminderToDelete = prisma.workflowReminder.deleteMany({
where: {
id: reminder.id,
},
});
remindersToDelete.push(reminderToDelete);
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Unawaited cancellation promises for rescheduled booking reminders.

Same fire-and-forget pattern: forEach drops the promises from deleteScheduledEmailReminder / deleteScheduledSMSReminder. These should be awaited to ensure external API cancellations complete before the response is sent.

🐛 Proposed fix
-        bookingToReschedule.workflowReminders.forEach((reminder) => {
-          if (reminder.method === WorkflowMethods.EMAIL) {
-            deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
-          } else if (reminder.method === WorkflowMethods.SMS) {
-            deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
-          }
-        });
+        await Promise.all(
+          bookingToReschedule.workflowReminders.map((reminder) => {
+            if (reminder.method === WorkflowMethods.EMAIL) {
+              return deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
+            } else if (reminder.method === WorkflowMethods.SMS) {
+              return deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
+            }
+          })
+        );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
//cancel workflow reminders of previous booking
bookingToReschedule.workflowReminders.forEach((reminder) => {
if (reminder.scheduled && reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
const reminderToDelete = prisma.workflowReminder.deleteMany({
where: {
id: reminder.id,
},
});
remindersToDelete.push(reminderToDelete);
});
//cancel workflow reminders of previous booking
await Promise.all(
bookingToReschedule.workflowReminders.map((reminder) => {
if (reminder.method === WorkflowMethods.EMAIL) {
return deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
return deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
})
);
🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/bookings.tsx` around lines 487 - 494, The
cancellation of workflow reminders on bookingToReschedule currently uses forEach
and fires deleteScheduledEmailReminder/deleteScheduledSMSReminder without
awaiting them, so ensure these async cancellations complete by collecting and
awaiting the promises (e.g., map to promises and await Promise.all(...) or use a
for..of with await) when method === WorkflowMethods.EMAIL call
deleteScheduledEmailReminder(reminder.id, reminder.referenceId) and when ===
WorkflowMethods.SMS call deleteScheduledSMSReminder(reminder.id,
reminder.referenceId); update the code around
bookingToReschedule.workflowReminders to await those calls before returning the
response.

Comment on lines +211 to 218
//cancel workflow reminders of deleted workflow
scheduledReminders.forEach((reminder) => {
if (reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Unawaited async cancellation calls — fire-and-forget before cascade delete.

forEach drops the promises returned by deleteScheduledEmailReminder / deleteScheduledSMSReminder. The subsequent workflow.deleteMany (line 220) cascade-deletes the same WorkflowReminder rows concurrently. For SMS, deleteScheduledSMSReminder also deletes the DB record, creating a race with the cascade.

Consider awaiting all cancellation calls before the cascade delete:

🐛 Proposed fix: await cancellations before cascade delete
-        scheduledReminders.forEach((reminder) => {
-          if (reminder.method === WorkflowMethods.EMAIL) {
-            deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
-          } else if (reminder.method === WorkflowMethods.SMS) {
-            deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
-          }
-        });
+        await Promise.all(
+          scheduledReminders.map((reminder) => {
+            if (reminder.method === WorkflowMethods.EMAIL) {
+              return deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
+            } else if (reminder.method === WorkflowMethods.SMS) {
+              return deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
+            }
+          })
+        );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
//cancel workflow reminders of deleted workflow
scheduledReminders.forEach((reminder) => {
if (reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
});
//cancel workflow reminders of deleted workflow
await Promise.all(
scheduledReminders.map((reminder) => {
if (reminder.method === WorkflowMethods.EMAIL) {
return deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
} else if (reminder.method === WorkflowMethods.SMS) {
return deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
})
);
🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/workflows.tsx` around lines 211 - 218,
The cancellation calls to deleteScheduledEmailReminder and
deleteScheduledSMSReminder are being fire-and-forget inside
scheduledReminders.forEach, creating a race with the subsequent
workflow.deleteMany cascade; collect the promises (e.g., map scheduledReminders
to calls to deleteScheduledEmailReminder/deleteScheduledSMSReminder) and await
Promise.all on them before calling workflow.deleteMany so all external scheduler
cancellations complete first; ensure you reference WorkflowMethods,
deleteScheduledEmailReminder, deleteScheduledSMSReminder and the
workflow.deleteMany call when making the change so the SMS path which also
removes DB rows won't race with the cascade delete.

Comment on lines +375 to 382
//cancel workflow reminders for all bookings from event types that got disabled
remindersToDelete.flat().forEach((reminder) => {
//already scheduled reminders
if (reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
const deleteReminder = ctx.prisma.workflowReminder.deleteMany({
where: {
id: reminder.id,
booking: {
userId: ctx.user.id,
},
},
});
deleteReminderPromise.push(deleteReminder);
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Same fire-and-forget issue for removed event type reminders.

Same pattern: forEach drops the returned promises. These use the default (non-immediateDelete) path which sets cancelled = true, so the DB write may silently fail if not awaited.

🐛 Proposed fix
-      remindersToDelete.flat().forEach((reminder) => {
-        if (reminder.method === WorkflowMethods.EMAIL) {
-          deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
-        } else if (reminder.method === WorkflowMethods.SMS) {
-          deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
-        }
-      });
+      await Promise.all(
+        remindersToDelete.flat().map((reminder) => {
+          if (reminder.method === WorkflowMethods.EMAIL) {
+            return deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
+          } else if (reminder.method === WorkflowMethods.SMS) {
+            return deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
+          }
+        })
+      );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
//cancel workflow reminders for all bookings from event types that got disabled
remindersToDelete.flat().forEach((reminder) => {
//already scheduled reminders
if (reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
const deleteReminder = ctx.prisma.workflowReminder.deleteMany({
where: {
id: reminder.id,
booking: {
userId: ctx.user.id,
},
},
});
deleteReminderPromise.push(deleteReminder);
});
//cancel workflow reminders for all bookings from event types that got disabled
await Promise.all(
remindersToDelete.flat().map((reminder) => {
if (reminder.method === WorkflowMethods.EMAIL) {
return deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
return deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
})
);
🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/workflows.tsx` around lines 375 - 382,
The current forEach on remindersToDelete drops promises from
deleteScheduledEmailReminder/deleteScheduledSMSReminder (used with the default
non-immediateDelete path that sets cancelled = true), which can cause the DB
write to be lost; replace the forEach with mapping to an array of promises and
await them (e.g. await Promise.all(remindersToDelete.flat().map(...))) so that
calls to deleteScheduledEmailReminder and deleteScheduledSMSReminder are awaited
and complete before continuing; keep using WorkflowMethods to branch but ensure
you return the promise from each call so Promise.all receives them.

Comment on lines 516 to 526
if (!newStep) {
//delete already scheduled reminders
// cancel all workflow reminders from deleted steps
if (remindersFromStep.length > 0) {
remindersFromStep.forEach((reminder) => {
if (reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
});
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Same fire-and-forget pattern for deleted step reminders.

forEach at line 519 drops promises. Additionally, the step is cascade-deleted immediately after (line 527), racing with deleteScheduledSMSReminder's DB delete.

🐛 Proposed fix
           if (remindersFromStep.length > 0) {
-            remindersFromStep.forEach((reminder) => {
-              if (reminder.method === WorkflowMethods.EMAIL) {
-                deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
-              } else if (reminder.method === WorkflowMethods.SMS) {
-                deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
-              }
-            });
+            await Promise.all(
+              remindersFromStep.map((reminder) => {
+                if (reminder.method === WorkflowMethods.EMAIL) {
+                  return deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
+                } else if (reminder.method === WorkflowMethods.SMS) {
+                  return deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
+                }
+              })
+            );
           }
🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/workflows.tsx` around lines 516 - 526,
The current fire-and-forget loop over remindersFromStep using forEach drops
promises and races with the cascade delete; replace the forEach with building an
array of promises (map each reminder to calling deleteScheduledEmailReminder or
deleteScheduledSMSReminder) and await Promise.all(promises) before proceeding to
cascade-delete the step (ensure the surrounding function is async and any call
sites handle the awaited result); reference remindersFromStep, newStep,
WorkflowMethods, deleteScheduledEmailReminder, and deleteScheduledSMSReminder
when making this change.

Comment on lines +572 to 580

//cancel all workflow reminders from steps that were edited
remindersToUpdate.forEach(async (reminder) => {
if (reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
await ctx.prisma.workflowReminder.deleteMany({
where: {
id: reminder.id,
},
});
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Same fire-and-forget pattern for edited step reminders.

🐛 Proposed fix
-          remindersToUpdate.forEach(async (reminder) => {
-            if (reminder.method === WorkflowMethods.EMAIL) {
-              deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
-            } else if (reminder.method === WorkflowMethods.SMS) {
-              deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
-            }
-          });
+          await Promise.all(
+            remindersToUpdate.map((reminder) => {
+              if (reminder.method === WorkflowMethods.EMAIL) {
+                return deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
+              } else if (reminder.method === WorkflowMethods.SMS) {
+                return deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
+              }
+            })
+          );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
//cancel all workflow reminders from steps that were edited
remindersToUpdate.forEach(async (reminder) => {
if (reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
await ctx.prisma.workflowReminder.deleteMany({
where: {
id: reminder.id,
},
});
});
//cancel all workflow reminders from steps that were edited
await Promise.all(
remindersToUpdate.map((reminder) => {
if (reminder.method === WorkflowMethods.EMAIL) {
return deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
return deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
})
);
🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/workflows.tsx` around lines 572 - 580,
The current remindersToUpdate.forEach(async (reminder) => { ... }) uses an async
callback without awaiting the promises, causing fire-and-forget behavior and
unhandled errors; change this to either await
Promise.all(remindersToUpdate.map(async reminder => { ... })) or use a for...of
loop and await each call, and wrap
deleteScheduledEmailReminder/deleteScheduledSMSReminder invocations in try/catch
to surface/log errors (refer to remindersToUpdate,
WorkflowMethods.EMAIL/WorkflowMethods.SMS, deleteScheduledEmailReminder,
deleteScheduledSMSReminder).

Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant